Skip to content

Add JNI-style global references and complete GC roots - #182

Merged
dlunch merged 1 commit into
mainfrom
agent/jni-global-references
Jul 18, 2026
Merged

Add JNI-style global references and complete GC roots#182
dlunch merged 1 commit into
mainfrom
agent/jni-global-references

Conversation

@dlunch

@dlunch dlunch commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a typed, RAII-managed GlobalRef<T> backed by independent JVM global-reference entries
  • keep object arguments and field, static, array, method, and exception results alive as frame-local references
  • use a global reference while Thread.start() waits for its host spawn callback to attach
  • release bootstrap-only local references after JVM initialization

Root cause

Thread.start() captured its Java Thread instance in a Rust callback, but the JVM garbage collector could not see Rust-owned references. If collection ran after the caller frame returned and before the spawned task attached, the thread object could be collected. The same local-reference gap existed for object values entering a frame through several JVM APIs.

Validation

  • cargo test --workspace
  • cargo test -p jvm --test test_garbage_collection
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • git diff --check

Copilot AI review requested due to automatic review settings July 18, 2026 03:13
@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.09091% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.11%. Comparing base (258cd4e) to head (7617a5a).

Files with missing lines Patch % Lines
java_runtime/src/classes/java/lang/thread.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #182      +/-   ##
==========================================
+ Coverage   87.03%   87.11%   +0.08%     
==========================================
  Files         198      199       +1     
  Lines       17846    17947     +101     
==========================================
+ Hits        15532    15635     +103     
+ Misses       2314     2312       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the JVM’s GC root tracking to cover Rust-owned references by introducing JNI-style global references and by treating certain API results/arguments as frame-local roots, addressing a real lifetime gap around Thread.start() and other object flows across native/Java boundaries.

Changes:

  • Add RAII-managed GlobalRef<T> backed by a global-reference table that the GC scans as a root set.
  • Record object arguments and returned object/exception results as frame-local references so they remain GC-visible across API boundaries.
  • Update Thread.start() to hold a global reference until the spawned callback attaches; add targeted GC regression tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test_utils/src/lib.rs Adds a queued-spawn mode to deterministically delay spawn callbacks in tests.
jvm/tests/test_garbage_collection.rs Adds tests covering global refs, frame-local roots for various APIs, and Thread.start() lifetime.
jvm/src/thread.rs Updates Java-frame creation to seed GC-visible locals from call arguments.
jvm/src/lib.rs Wires in the new global_ref module and exports GlobalRef.
jvm/src/jvm.rs Implements global refs, clears bootstrap locals post-init, and roots returned object/exception values + certain API results.
jvm/src/global_ref.rs Introduces GlobalReferences + RAII GlobalRef<T> with Drop cleanup.
jvm/src/garbage_collector.rs Treats global references as additional GC roots.
java_runtime/src/classes/java/lang/thread.rs Uses GlobalRef<Thread> to keep the thread instance alive until the spawn callback runs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jvm/src/thread.rs
@dlunch
dlunch merged commit e557673 into main Jul 18, 2026
11 checks passed
@dlunch
dlunch deleted the agent/jni-global-references branch July 18, 2026 05:02
Jun025 added a commit to Jun025/RustJava that referenced this pull request Jul 31, 2026
Judged the two remaining remote branches on the fork:

- dependabot/cargo/tracing-attributes-0.1.31: deleted. PR #4 (fa92ef9)
  removed the tracing-attributes direct dependency outright, so the
  branch patches a Cargo.toml line that no longer exists.
- wie-ktf-hardening: preserved. 8 of its 12 commits are already in
  upstream/main via squash merges (dlunch#174 dlunch#175 dlunch#176 dlunch#177 dlunch#180 dlunch#182);
  git cherry missed this because origin/main trails upstream/main by
  20 commits. 4 commits carry residual value.

No code changes.

Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com>
Co-authored-by: Claude <noreply@anthropic.com>
Jun025 added a commit to Jun025/RustJava that referenced this pull request Aug 27, 2026
* Bump bytemuck from 1.25.0 to 1.25.1 (dlunch#173)

Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.25.0 to 1.25.1.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.25.0...v1.25.1)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.25.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Replace runtime panics with the matching Java exceptions (dlunch#174)

* Replace runtime panics with the matching Java exceptions

An unwrap audit found panics reachable from ordinary Java code:

- File.length() returns 0 for a missing file; isDirectory/isFile lose
  their guard-then-unwrap shape
- FileImpl (native runtime) maps open/read/write/seek failures to
  IOError instead of panicking, so FileInputStream and RandomAccessFile
  guards actually produce FileNotFoundException; FileOutputStream gains
  the same guard
- File I/O operations (read/write/seek/available/length/setLength)
  throw java.io.IOException on failure via a shared helper
- Class.forName resolves the class and throws ClassNotFoundException
  (new runtime class) instead of panicking on any not-yet-loaded name
- StringBuffer.append(char)/append(char[]) keep exact UTF-16 units so
  unpaired surrogates no longer panic and pairs built char by char
  survive; String.valueOf(char) builds through [C for the same reason
- PrintStream.println(char) replaces an unpaired surrogate with '?'
  like the JDK charset encoder
- ZipFile validates the archive in its constructor and throws
  java.util.zip.ZipException (new runtime class) for a malformed
  archive; getInputStream returns null for a missing entry

Expected outputs for the new fixtures are generated by a real JVM.
Remaining unwraps are invariants (interpreter stack discipline, thread
attach), guarded lookups, or documented gaps (lenient calendar
normalization, ClassFormatError plumbing).

* Inline the IOException conversion at each I/O call site

* Return the same Thread object from Thread.currentThread() (dlunch#175)

Every attached thread now owns its java/lang/Thread instance: attach
takes the instance for threads started via Thread.start (so
currentThread() inside run() is the started Thread object) and creates
one otherwise (bootstrap, external attachers). currentThread() returns
the stored instance, and the GC roots it per thread.

Also parse unrecognized classfile attributes as an opaque Unknown
variant instead of failing — JVMS 4.7.1 requires silently ignoring
them, and the anonymous-class fixture carries EnclosingMethod and
Signature attributes the parser rejected.

Expected output for the fixture is generated by a real JVM.

* Add Java primitive wrapper classes (dlunch#176)

* Add Java primitive wrapper classes

* Use Character digit semantics for numeric parsing

* Bump tokio from 1.52.3 to 1.52.4 (dlunch#179)

Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.52.3 to 1.52.4.
- [Release notes](https://github.com/tokio-rs/tokio/releases)
- [Commits](tokio-rs/tokio@tokio-1.52.3...tokio-1.52.4)

---
updated-dependencies:
- dependency-name: tokio
  dependency-version: 1.52.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add CLDC 1.1 core API compatibility (dlunch#177)

* Add CLDC 1.1 core API compatibility

* Fix CI lint and improve CLDC coverage

* Fix array assignability and reader progress

* Harden JVM runtime correctness (dlunch#180)

* Harden JVM runtime correctness

* Address classfile review findings

* Move class initialization tests to Java fixture

* Separate classfile validation from JVM verification

* Remove ClassFileError re-export

* Bump tokio from 1.52.4 to 1.53.0 (dlunch#181)

Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.52.4 to 1.53.0.
- [Release notes](https://github.com/tokio-rs/tokio/releases)
- [Commits](tokio-rs/tokio@tokio-1.52.4...tokio-1.53.0)

---
updated-dependencies:
- dependency-name: tokio
  dependency-version: 1.53.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Hide classfile errors behind class definition errors

* Delegate null-parent class loading to bootstrap

* Remove duplicate array instance methods

* Add JNI-style global references (dlunch#182)

* Generalize monitor instance arguments

* Add CDC text formatting APIs (dlunch#183)

* Add CDC text formatting APIs

* Add integer number format factories

* Fix text format position handling

* Add CLI classpath options (dlunch#184)

* Add CLI classpath options

* Simplify URL classpath lookup

* Fix platform classpath handling

* Use File path separator for class loading

* Separate RustJar class loading

* [rustjava-upstream-sync-s4] test: widen test_timer_periodic margin 500ms->2000ms (upstream 3296139 slowed TimerThread)

* [rustjava-upstream-sync-s4] docs: record S4 landing (cut 3296139, conflicts 20->2) + S3 landing sha

* [rustjava-upstream-sync-s4] docs: correct the timer finding - chronic boundary test, not a cut regression

The prior wording compared a standalone run on origin/main against parallel full-suite runs on
upstream and called the gap a regression. Matched-condition alternating runs show no difference
(standalone x10: pre 3.5 mean / post 3.5 mean; full-suite x8: no difference). Upstream widened
this same margin in 895d67d (2025-08) and ad8b477 (2025-10), both already ancestors of main,
11 months before e557673 (2026-07) which the prior wording blamed.

sleep 2000 and both conflict resolutions are untouched. Comment-only in .rs; no code change.

* [rustjava-upstream-sync-s4] docs: retarget follow-up (4) - no timer perf regression exists; the open axis is our test's wall-clock dependence

REPORT.md line 27 already said the 'imported upstream regression' framing was wrong, but the
follow-up list 20 lines below still carried it verbatim - and that list is what the next round
tickets from. Retargeted to the axis that does exist (our test design, no upstream sending).

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Inseok Lee <git@dlun.ch>
Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants